test(robot): add block disk back to previously deleted node for v2 test cases #2190
Conversation
…st cases Signed-off-by: Yang Chiu <[email protected]>
WalkthroughThe pull request introduces enhancements to Kubernetes-related keywords in the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
e2e/keywords/k8s.resource (1)
34-36: Consider enhancing the keyword implementation
- Add documentation to describe the keyword's purpose and parameters
- Consider moving the disk paths to constants
- Use more descriptive variable names (e.g.,
block_disk_pathinstead ofdisk_path)+ [Documentation] Adds a block disk back to a previously deleted node + + *** Variables *** + ${HARVESTER_BLOCK_DISK_PATH} /dev/vdc + ${DEFAULT_BLOCK_DISK_PATH} /dev/xvdh + Add deleted node back reboot_node_by_name ${deleted_node} ${host_provider}= Get Environment Variable HOST_PROVIDER - ${disk_path}= Set Variable If "${host_provider}" == "harvester" /dev/vdc /dev/xvdh + ${block_disk_path}= Set Variable If "${host_provider}" == "harvester" ${HARVESTER_BLOCK_DISK_PATH} ${DEFAULT_BLOCK_DISK_PATH} - add_disk block-disk ${deleted_node} block ${disk_path} + add_disk block-disk ${deleted_node} block ${block_disk_path}e2e/libs/node/node.py (1)
65-77: Enhance method implementation and fix static analysis warningThe retry mechanism is well implemented, but consider these improvements:
- Fix the unused loop variable warning
- Add method documentation
- Add type hints for better code maintainability
- def add_disk(self, node_name, disk): + def add_disk(self, node_name: str, disk: dict) -> None: + """Add a disk to a node with retry mechanism. + + Args: + node_name: Name of the node to add the disk to + disk: Dictionary containing disk configuration + + Raises: + AssertionError: If adding the disk fails after all retries + """ added = False - for i in range(self.retry_count): + for _ in range(self.retry_count): try: node = get_longhorn_client().by_id_node(node_name) disks = node.disks🧰 Tools
🪛 Ruff (0.8.2)
66-66: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
e2e/keywords/k8s.resource(1 hunks)e2e/libs/node/node.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
e2e/libs/node/node.py
66-66: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🔇 Additional comments (1)
e2e/libs/node/node.py (1)
65-77: Implementation aligns well with class design
The retry mechanism implementation is consistent with the class's error handling patterns and properly utilizes the existing retry infrastructure.
🧰 Tools
🪛 Ruff (0.8.2)
66-66: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
Which issue(s) this PR fixes:
Issue longhorn/longhorn#10001
What this PR does / why we need it:
add block disk back to previously deleted node for v2 test cases
Special notes for your reviewer:
Additional documentation or context
Summary by CodeRabbit